Repository navigation
fix(agc): read user-data callees up to the 1 MiB raw shader program limit - #10140
Conversation
github-actions
Bot
added
the
unblocks title
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The reader change raises the bound to the existing raw-program 1 MiB limit and replaces per-page full rescans with geometric checks; it still checks for a return before reporting an unreadable next page and retains the bounded no-return error. The regression allocates 20,000 dwords across committed pages followed by an unreadable page and checks both registered/unregistered dispatch paths where supported. The author reports repeated focused CTest passes, explicit main-control failures for the new long-callee cases, unchanged-main controls, and validation-layer runs on Windows/RTX 4090. Those runtime results are author-reported; I could not run the GPU test locally, and a local syntax-only compile was blocked by absent decoder/SPIR-V development headers. In the exact worktree, conventions, diff-check, and main-sync checks pass (1 ahead/4 behind current main; merge clean). Hosted eligibility/intake/conventions/labels pass; product jobs are skipped by the workflow.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
A current overlap was missed in the checklist: open #8140 also edits ReadUserDataCallee in ShaderRegistry.cpp (adding the allowGetpc parameter and using it for table callees). I fetched its current head and pairwise git merge-tree reports a content conflict in this same function. The behavior is distinct, but whichever merges second must reconcile both changes; please coordinate the order with #8140’s author and record the integration dependency/overlap in the checklist. This is required by the repository overlap rule. Your local checks and reported RTX 4090 focused runs remain relevant; I did not run a GPU test locally.
|
Windows run on a second NVIDIA GPU and on Intel. Windows 11 Pro 10.0.26300, WinLibs GCC 15.2.0 r7, Release. NVIDIA GeForce RTX 3060 Ti (driver 617.42) and Intel Arc Graphics (Meteor Lake, driver 101.9150), selected with
AI-assisted: yes (Claude Code) |
|
@MarcZX10 thanks, I'd missed #8140. The checklist now records the overlap, and I've proposed an order on #8140: it lands first, then I merge AI-assisted: yes (Claude Code) |
|
AMD run on Windows, to go with the NVIDIA and Intel ones. Windows 11 10.0.26300, WinLibs GCC 15.2.0, Release;
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
Overlap with #11442 in #11442 is small:
Its review only asks for a sync with current Once we agree, I'll record the order with |
✅ NVIDIA GPU test: passedPR head
Intermittent validation messages (1, seen once in the parallel run, not reproduced when the test was re-run alone 2 times; not counted against this PR)Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (4, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validation IDs already present on main (4 IDs in 404 tests)Skipped (39)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Conflicts: - core/libs/prx/libSceAgcDriver/CMakeLists.txt: main moved AGC test registrations into tests/cmake fragments (00dd261). Resolved with tools/agc_test_split.py resolve; agc_driver_queued_shader_calls keeps ANYPS5_NO_SHADER_CACHE=1;APS5_PIPELINED_DRAWS=8 and TIMEOUT 30 in tests/cmake/agc_driver_queued_shader_calls_tests.cmake. - ShaderRegistry.cpp: ReadUserDataCallee keeps main's boykopovar#10140 early return on an unreadable page, with DrainShaderWrites ahead of it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
ReadUserDataCallee(6d9321f) reads a compute shader's user-data or table callee from guest memory until it can prove the return, and throws once it has read 16,384 dwords (64 KiB) without one. Persona 3 Reload (PPSA10872) calls a 17,369-dword callee from compute shader 0x2014960000, so the dispatch throwsits callee at ... does not return within 65536 bytesbefore the shader is recompiled; @zaidhawk raised the limit to 32,768 dwords locally to get past it (#9068 comment).ShaderRegistryalready uses for the raw shader program it reads at the same address range (limit = 1024 * 1024in the snapshot reader). A callee that does not return within it still throws the same message with the new size, and an unreadable page before the return still throwsis not readable at.UserDataCalleeLengthdecodes the callee from its start each time it is asked, and the reader asked after every 4 KiB page, so reading n dwords cost O(n²/1024) decoded dwords. With a window 16 times larger, the reader now asks when the read size has doubled since the last check, at the limit, and when the next page is unreadable, so a callee costs at most about twice its length in decoded dwords. The result is the same:UserDataCalleeLengthreturns at the firsts_setpc_b64through the link register, so words read past it never change the answer, and the checks before an unreadable page and at the limit keep every callee that returned before.Tested
Windows 11 (build 26200), NVIDIA GeForce RTX 4090 (driver 616.92), MinGW GCC (WinLibs 15.2) Release build with
BUILD_TESTING=ON.agc_driver_compute_user_data_calls(both modes, plain and--registered, wave32 and wave64 where the test runs them): a 20,000-dword callee (v_add_nc_u32 v1, 1, v1, 19,998s_nop 0,s_setpc_b64 s[14:15]) in 20 committed pages of a reservation whose 21st page is not committed, called with the existing 64-adding callee: every lane gets 3 x (1 + 64). The return lies in the last page, which is read after the last size check, so the case also covers the check before an unreadable page.agc_driver_compute_user_data_calls(2),agc_driver_compute_table_calls,agc_driver_compute_fs_table_callsandscalar_call:main513ca7e): 5/5, each test run three times (ctest --repeat until-fail:3).main513ca7e with only this PR'sComputeUserDataCalls.cpp: bothagc_driver_compute_user_data_callsentries fail withmain'sits callee at 0x1f044a90000 does not return within 65536 bytes; the other three pass.main513ca7e: 5/5.VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(Vulkan SDK 1.4.363.0),ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1withctest -V, on this PR and onmain513ca7e: 5/5 each, neither log haswas not found but was requested by env var VK_INSTANCE_LAYERS, and neither has aVUID-line or any other validation message.tools/check_conventions.py --base origin/mainpasses.Checklist
main; no other open PR implements the same functions. fix(shader): link table callees that read their program counter #8140 (tv7) also editsReadUserDataCallee, for a different change (it lets table callees read their program counter); fix(shader): link table callees that together exceed s_call_b64 range #9068 and fix(shader): link a table call whose V# is a slice of an s_load_dwordx8 or s_load_dwordx16 #8070 change how callees are linked and matched, not how far one is read.NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)mainafter it and keeps itsDrainShaderWritescalls). Also overlaps fix(shader): link table callees that read their program counter #8140 inReadUserDataCallee: proposed order fix(shader): link table callees that read their program counter #8140 first, then this PR mergesmainand passes fix(shader): link table callees that read their program counter #8140'sallowGetpcthroughfinishtoUserDataCalleeLength(asked on fix(shader): link table callees that read their program counter #8140).